[IA64] fix GNTTABOP_replace_and_unmap
authorIsaku Yamahata <yamahata@valinux.co.jp>
Fri, 25 Apr 2008 11:13:52 +0000 (20:13 +0900)
committerIsaku Yamahata <yamahata@valinux.co.jp>
Fri, 25 Apr 2008 11:13:52 +0000 (20:13 +0900)
This patch fixes the following xen panic repored by Akio Takebe.
> When we tested network between domU <-> dom0 with FTP load tools,
> we hitted BUG() in hypervisor. It is always reproducible for a few minutes.
> At that time, we got the following message.
> vmi15.sky.yk.fujitsu.co.jp login: (XEN) Xen BUG at mm.c:1254
>
> (XEN) FIXME: implement ia64 dump_execution_state()
> (XEN)
> (XEN) ****************************************
> (XEN) Panic on CPU 0:
> (XEN) Xen BUG at mm.c:1254
> (XEN) ****************************************
> (XEN)
> (XEN) Manual reset required ('noreboot' specified)
> (XEN) machine_halt called.  spinning...

GNTTABOP_replace_and_unmap must updates both the p2m table and m2p
table. However the m2p table update was missing so that
the above BUG_ON() was triggered detecting the inconsistency
between the p2m table and the m2p table.
This patch adds the missing the m2p table updates.
This patch also fixes the error path of the function. It may
return before completing the page table manipulation.

Signed-off-by: Isaku Yamahata <yamahata@valinux.co.jp>
xen/arch/ia64/xen/mm.c

index bed11ac04c605682451c851f7c0307b425482faf..7f82eece617686c6d92e9183365affb38ac84e27 100644 (file)
@@ -2187,6 +2187,7 @@ replace_grant_host_mapping(unsigned long gpaddr,
     struct page_info* page = mfn_to_page(mfn);
     struct page_info* new_page = NULL;
     volatile pte_t* new_page_pte = NULL;
+    unsigned long new_page_mfn;
 
     if (new_gpaddr) {
         new_page_pte = lookup_noalloc_domain_pte_none(d, new_gpaddr);
@@ -2194,7 +2195,6 @@ replace_grant_host_mapping(unsigned long gpaddr,
             new_pte = ptep_get_and_clear(&d->arch.mm,
                                          new_gpaddr, new_page_pte);
             if (likely(pte_present(new_pte))) {
-                unsigned long new_page_mfn;
                 struct domain* page_owner;
 
                 new_page_mfn = pte_pfn(new_pte);
@@ -2255,22 +2255,24 @@ replace_grant_host_mapping(unsigned long gpaddr,
         goto out;
     }
 
-    old_pte = ptep_cmpxchg_rel(&d->arch.mm, gpaddr, pte, cur_pte, new_pte);
-    if (unlikely(!pte_present(old_pte))) {
-        gdprintk(XENLOG_INFO, "%s: gpaddr 0x%lx mfn 0x%lx"
-                         " cur_pte 0x%lx old_pte 0x%lx\n",
-                __func__, gpaddr, mfn, pte_val(cur_pte), pte_val(old_pte));
-        goto out;
+    if (new_page) {
+        set_gpfn_from_mfn(new_page_mfn, gpfn);
+        /* smp_mb() isn't needed because assign_domain_pge_cmpxchg_rel()
+           has release semantics. */
     }
+    old_pte = ptep_cmpxchg_rel(&d->arch.mm, gpaddr, pte, cur_pte, new_pte);
     if (unlikely(pte_val(cur_pte) != pte_val(old_pte))) {
         if (pte_pfn(old_pte) == mfn) {
             goto again;
         }
-        gdprintk(XENLOG_INFO, "%s gpaddr 0x%lx mfn 0x%lx cur_pte "
-                "0x%lx old_pte 0x%lx\n",
-                __func__, gpaddr, mfn, pte_val(cur_pte), pte_val(old_pte));
+        if (new_page) {
+            set_gpfn_from_mfn(new_page_mfn, INVALID_M2P_ENTRY);
+            domain_put_page(d, new_gpaddr, new_page_pte, new_pte, 1);
+        }
         goto out;
     }
+    if (unlikely(!pte_present(old_pte)))
+        goto out;
     BUG_ON(pte_pfn(old_pte) != mfn);
 
     /* try_to_clear_PGC_allocate(d, page) is not needed. */
@@ -2283,8 +2285,9 @@ replace_grant_host_mapping(unsigned long gpaddr,
     return GNTST_okay;
 
  out:
-    if (new_page)
-        domain_put_page(d, new_gpaddr, new_page_pte, new_pte, 1);
+    gdprintk(XENLOG_INFO, "%s gpaddr 0x%lx mfn 0x%lx cur_pte "
+             "0x%lx old_pte 0x%lx\n",
+             __func__, gpaddr, mfn, pte_val(cur_pte), pte_val(old_pte));
     return GNTST_general_error;
 }